Skip to content

Conversation

@QuantumExplorer
Copy link
Member

@QuantumExplorer QuantumExplorer commented Mar 13, 2025

Summary by CodeRabbit

  • New Features

    • Enhanced token reward distribution with more robust cycle management and improved error feedback for token claims.
    • Introduced saturation control for token minting, offering better management of supply adjustments.
    • Added new methods for fetching last paid timestamps for pre-programmed distributions.
  • Improvements

    • Streamlined internal logic for token operations and updated system limits to support more efficient token redemption.
    • Refined payout processes and reward calculations for greater consistency and reliability.
    • Expanded handling of token distribution types and improved error handling for claims.
  • Tests

    • Expanded comprehensive test coverage for various token distribution and minting scenarios, ensuring robust performance.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 13, 2025

Caution

Review failed

The head commit changed during the review from 3df98ab to 9bd1d2c.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🔭 Outside diff range comments (1)
packages/rs-drive/src/drive/tokens/distribution/mark_pre_programmed_release_as_distributed/mod.rs (1)

38-49: ⚠️ Potential issue

Parameter changes need documentation update

The method signature has been updated to use recipient_id instead of the previous parameters, but the function documentation (lines 22-24) still references the old parameters owner_id and identity_id which no longer exist in the signature.

Update the documentation to reflect the new signature:

/// # Parameters
/// 
/// - `token_id`: The unique 32‑byte identifier of the token.
-/// - `owner_id`: The unique 32‑byte identifier of the owner initiating the distribution.
-/// - `identity_id`: The 32‑byte identity identifier for which the pre‑programmed release was scheduled.
+/// - `recipient_id`: The unique 32‑byte identifier of the recipient of the distribution.
/// - `release_time`: The scheduled release time (in milliseconds).
🧹 Nitpick comments (23)
packages/rs-platform-version/src/version/system_limits/v1.rs (1)

12-12: New system limit added for token redemption cycles

Adding a maximum limit for token redemption cycles is important to prevent potential abuse or resource exhaustion in token distribution processes.

Consider adding a documentation comment explaining what token redemption cycles are and why the limit is set to 128. This would make the code more maintainable and help future developers understand the purpose of this limit.

 pub const SYSTEM_LIMITS_V1: SystemLimits = SystemLimits {
     estimated_contract_max_serialized_size: 16384,
     max_field_value_size: 5120,       //5 KiB
     max_state_transition_size: 20480, //20 KiB
     max_transitions_in_documents_batch: 1,
     withdrawal_transactions_per_block_limit: 4,
     retry_signing_expired_withdrawal_documents_per_block_limit: 1,
     max_withdrawal_amount: 50_000_000_000_000, //500 Dash
     max_contract_group_size: 256,
+    // Maximum number of token redemption cycles allowed to prevent resource exhaustion
     max_token_redemption_cycles: 128,
 };
packages/rs-drive/src/drive/tokens/distribution/mod.rs (1)

15-16: Module commented out instead of being removed.

The module set_perpetual_distribution_next_event_for_identity_id has been commented out rather than properly removed. This could lead to confusion for future maintainers as commented out code is often overlooked during refactoring.

Consider properly removing the commented lines if this module is no longer needed:

-// #[cfg(feature = "server")]
-// mod set_perpetual_distribution_next_event_for_identity_id;

Alternatively, if this is temporary, add a comment explaining why it's commented out rather than removed.

packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/methods/v0/mod.rs (2)

24-25: Outdated comment after return type change

The comment on line 24 states "we use u64 as a catch-all for any type of interval", but the method now returns RewardDistributionMoment instead of u64.

Update the comment to reflect the new return type:

-    /// we use u64 as a catch-all for any type of interval we might have, eg time, block or epoch
+    /// Returns the next distribution moment based on the provided block information

27-27: New method for retrieving current interval

The addition of the current_interval method complements the existing next_interval method, providing a more complete API.

Consider adding a documentation comment for this method to maintain consistency with the rest of the API:

+    /// Returns the current distribution moment based on the provided block information
     fn current_interval(&self, block_info: &BlockInfo) -> RewardDistributionMoment;
packages/rs-drive/src/drive/tokens/distribution/fetch/pre_programmed_distribution_last_paid_time_ms/v0/mod.rs (1)

14-32: Revisit the doc comment to align it with the actual return type.

The doc references returning Some(RewardDistributionMoment) but the code actually returns Some(TimestampMillis) as a u64. Updating the documentation will improve accuracy and prevent confusion for future maintainers.

packages/rs-drive/src/drive/tokens/estimated_costs/for_token_pre_programmed_distribution/v0/mod.rs (1)

27-29: Documentation needs update for the optional times parameter

The parameter type has changed to Option<I>, but the documentation on line 25 hasn't been updated to reflect that this parameter is now optional.

Update the documentation to reflect that times is now optional:

/// # Parameters
/// 
/// - `token_id`: The identifier for the token.
-/// - `times`: A vector of timestamps (in milliseconds) for which pre-programmed distributions exist.
+/// - `times`: An optional vector of timestamps (in milliseconds) for which pre-programmed distributions exist.
/// - `estimated_costs_only_with_layer_info`: A mutable hashmap that holds estimated layer information.
packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/evaluate.rs (2)

9-10: Consider providing a usage example in the doc comments.
Adding simple examples helps users quickly understand how to pass the new contract_registration_step parameter.


114-156: Refactored linear function logic is comprehensive.

  1. The use of saturating_sub and careful overflow checks are commendable.
  2. Check that large negative slopes do not cause repeated clamping to 0 if a lower bound is not provided.
packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_claim_transition_action/v0/transformer.rs (1)

2-2: Avoid unused import std::process::id; check if it’s required.
If not used, removing it reduces clutter.

packages/rs-drive/src/drive/tokens/paths.rs (1)

217-228: Fix doc comment for pre-programmed distribution function.

The doc comment says "The path for the token perpetual distributions tree," but this function actually returns the path for the pre-programmed distribution last-claimed time. Update the doc comment to reflect the correct functionality.

packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/pre_programmed.rs (2)

14-187: Improve reuse of setup logic in first test.

In test_token_pre_programmed_distribution_two_claims, the setup patterns (creating identities, contracts, and performing claims) are repeated across other tests. Extracting common setup steps into a helper method may reduce duplication and enhance maintainability.


750-870: Consider clarifying combination of distributions.

The final test (test_token_pre_programmed_distribution_claim_no_pre_programmed_rewards_for_recipient_when_they_have_perpetual) checks mixed distribution rules. It might be beneficial to add explicit inline comments explaining why the perpetual rewards do not affect the pre-programmed claims or vice versa, to minimize confusion for future maintainers.

packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_type/mod.rs (1)

139-193: New max_cycle_moment method.
This saturating approach for capping reward distribution is logical. If partial overflows must be detected rather than silently clamped, consider using checked arithmetic in future iterations.

packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_moment/mod.rs (1)

492-665: Extensive test coverage.
Mismatched distributions, zero-step, and boundary checks are well-covered. Consider adding tests for near-overflow scenarios to provide comprehensive arithmetic validation.

packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual/time_based.rs (1)

503-690: Potential improvement for repetitive test code.
This test function nearly duplicates the structure in other tests (e.g., creating a claim transition, processing it, checking balances). Consider extracting a helper function or macro to reduce duplication and improve maintainability across similar tests.

packages/rs-drive/src/drive/tokens/mint/mod.rs (1)

21-34: Add documentation for allow_saturation.
The new parameter is introduced but not documented within the method doc comment. Provide a brief explanation of its significance to ensure clarity for future maintainers.

 /// Mints (issues) new tokens by increasing the total supply and adding them to an identity's balance.
-pub fn token_mint(
+/// 
+/// # Parameters
+/// * `allow_saturation` - If true, ...
+pub fn token_mint(
packages/rs-drive/src/drive/tokens/system/add_to_token_total_supply/mod.rs (2)

20-22: Consider documenting the new allow_saturation parameter.
Adding allow_saturation and switching amount to TokenAmount is a great step toward robust handling of large amounts. However, these changes would benefit from additional documentation explaining what saturation means in this context and why it’s sometimes desirable.

Also applies to: 27-27


57-59: Align parameter ordering and provide usage clarity in docs.
In add_to_token_total_supply_add_to_operations, the new allow_saturation parameter is also introduced. Confirm the parameter order matches the analogous function to minimize confusion, and ensure the doc comments clarify how to use these parameters effectively.

Also applies to: 64-64

packages/rs-drive/src/drive/tokens/system/add_to_token_total_supply/v0/mod.rs (3)

23-23: Consider robust validation when switching to TokenAmount.
Ensuring allow_saturation logic is well understood is crucial. Provide clarity on any upper bounds or edge cases that might trigger a saturating add versus an error.

You could, for instance, factor out the logic of saturating vs. non-saturating addition into a helper function for improved readability:

+fn add_amount(current: i64, amount: i64, allow_saturation: bool) -> Result<i64, DriveError> {
+    if allow_saturation {
+        Ok(current.saturating_add(amount))
+    } else {
+        current.checked_add(amount).ok_or_else(|| {
+            DriveError::CorruptedCodeExecution("overflow in non-saturating addition")
+        })
+    }
+}

Also applies to: 28-28, 31-31, 36-36, 51-51


68-68: Ensure minimal duplication of logic.
add_to_token_total_supply_operations_v0 is called here, and repeated logic about saturation is also performed in these lines. Consider centralizing or reusing code to unify overflow checks and operation building steps.

Also applies to: 72-72, 84-85


123-160: Thorough overflow handling.
The saturating vs. checked addition logic is clearly delineated. The code fails early if an overflow risk is detected with checked_add or if the minted amount exceeds the i64::MAX. This is a robust approach, but ensure you have:

  1. Adequate test coverage for boundary cases (e.g. near integer limits).
  2. Clear documentation about why i64::MAX is the cap.
packages/rs-drive/src/drive/tokens/distribution/add_pre_programmed_distribution/v0/mod.rs (1)

194-202: Added tracking for identity's last pre-programmed distribution claim.

Added a new tree insertion for tracking the last claim timestamp for each identity in pre-programmed distributions. This is an important addition that enables proper management of token distribution claims, preventing duplicate claims and maintaining distribution state.

This enhancement is part of a broader token distribution fix that ensures proper tracking of claims. Consider also implementing validation to ensure that claimed distributions are properly recorded to avoid potential race conditions.

packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/add_epoch_pool_to_proposers_payout_operations/mod.rs (1)

42-42: Switched from Vec to BTreeMap for proposer rewards.

Changed the return type from a vector of tuples Vec<(Identifier, u64)> to a BTreeMap<Identifier, u64> for storing proposer identifiers and their associated rewards.

This is a significant architectural improvement as BTreeMap provides:

  1. O(log n) lookup times compared to O(n) with vectors
  2. Automatic sorting of keys (identifiers)
  3. Deduplication of identifiers (prevents duplicate entries)
  4. More idiomatic access patterns through key-value pairs

This change likely requires corresponding updates in the implementation and any code that consumes this interface.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a2bba66 and ee41999.

📒 Files selected for processing (79)
  • packages/rs-dpp/src/block/finalized_epoch_info/getters.rs (2 hunks)
  • packages/rs-dpp/src/block/finalized_epoch_info/v0/getters.rs (3 hunks)
  • packages/rs-dpp/src/block/finalized_epoch_info/v0/mod.rs (2 hunks)
  • packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/accessors/mod.rs (0 hunks)
  • packages/rs-dpp/src/data_contract/associated_token/token_configuration_localization/mod.rs (0 hunks)
  • packages/rs-dpp/src/data_contract/associated_token/token_distribution_key.rs (3 hunks)
  • packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/encode.rs (15 hunks)
  • packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/evaluate.rs (57 hunks)
  • packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/evaluate_interval.rs (1 hunks)
  • packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/mod.rs (12 hunks)
  • packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/reward_ratio.rs (1 hunks)
  • packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/validation.rs (73 hunks)
  • packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/methods/mod.rs (2 hunks)
  • packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/methods/v0/mod.rs (2 hunks)
  • packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_moment/mod.rs (5 hunks)
  • packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_type/accessors.rs (0 hunks)
  • packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_type/evaluate_interval.rs (2 hunks)
  • packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_type/mod.rs (3 hunks)
  • packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/v0/methods.rs (1 hunks)
  • packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v0/mod.rs (0 hunks)
  • packages/rs-dpp/src/data_contract/document_type/property/mod.rs (1 hunks)
  • packages/rs-dpp/src/data_contract/v0/methods/schema.rs (0 hunks)
  • packages/rs-dpp/src/data_contract/v0/serialization/mod.rs (0 hunks)
  • packages/rs-dpp/src/data_contract/v1/methods/schema.rs (0 hunks)
  • packages/rs-dpp/src/data_contract/v1/serialization/mod.rs (0 hunks)
  • packages/rs-dpp/src/errors/consensus/codes.rs (2 hunks)
  • packages/rs-dpp/src/errors/consensus/state/state_error.rs (2 hunks)
  • packages/rs-dpp/src/errors/consensus/state/token/invalid_token_claim_no_current_rewards.rs (1 hunks)
  • packages/rs-dpp/src/errors/consensus/state/token/invalid_token_claim_wrong_claimant.rs (1 hunks)
  • packages/rs-dpp/src/errors/consensus/state/token/mod.rs (2 hunks)
  • packages/rs-dpp/src/errors/protocol_error.rs (1 hunks)
  • packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/add_epoch_pool_to_proposers_payout_operations/mod.rs (2 hunks)
  • packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/add_epoch_pool_to_proposers_payout_operations/v0/mod.rs (3 hunks)
  • packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/find_oldest_epoch_needing_payment/v0/mod.rs (0 hunks)
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/mod.rs (1 hunks)
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual.rs (0 hunks)
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual/block_based.rs (1 hunks)
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual/mod.rs (1 hunks)
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual/time_based.rs (1 hunks)
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/pre_programmed.rs (1 hunks)
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/mod.rs (40 hunks)
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/state/v0/mod.rs (0 hunks)
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_update/state/v0/mod.rs (0 hunks)
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/mod.rs (3 hunks)
  • packages/rs-drive/src/drive/contract/insert/insert_contract/v1/mod.rs (0 hunks)
  • packages/rs-drive/src/drive/initialization/v1/mod.rs (1 hunks)
  • packages/rs-drive/src/drive/tokens/distribution/add_perpetual_distribution/mod.rs (0 hunks)
  • packages/rs-drive/src/drive/tokens/distribution/add_perpetual_distribution/v0/mod.rs (2 hunks)
  • packages/rs-drive/src/drive/tokens/distribution/add_pre_programmed_distribution/v0/mod.rs (3 hunks)
  • packages/rs-drive/src/drive/tokens/distribution/fetch/mod.rs (1 hunks)
  • packages/rs-drive/src/drive/tokens/distribution/fetch/pre_programmed_distribution_last_paid_time_ms/mod.rs (1 hunks)
  • packages/rs-drive/src/drive/tokens/distribution/fetch/pre_programmed_distribution_last_paid_time_ms/v0/mod.rs (1 hunks)
  • packages/rs-drive/src/drive/tokens/distribution/mark_perpetual_release_as_distributed/mod.rs (2 hunks)
  • packages/rs-drive/src/drive/tokens/distribution/mark_perpetual_release_as_distributed/v0/mod.rs (1 hunks)
  • packages/rs-drive/src/drive/tokens/distribution/mark_pre_programmed_release_as_distributed/mod.rs (2 hunks)
  • packages/rs-drive/src/drive/tokens/distribution/mark_pre_programmed_release_as_distributed/v0/mod.rs (7 hunks)
  • packages/rs-drive/src/drive/tokens/distribution/mod.rs (1 hunks)
  • packages/rs-drive/src/drive/tokens/estimated_costs/for_token_perpetual_distribution/mod.rs (0 hunks)
  • packages/rs-drive/src/drive/tokens/estimated_costs/for_token_perpetual_distribution/v0/mod.rs (0 hunks)
  • packages/rs-drive/src/drive/tokens/estimated_costs/for_token_pre_programmed_distribution/mod.rs (2 hunks)
  • packages/rs-drive/src/drive/tokens/estimated_costs/for_token_pre_programmed_distribution/v0/mod.rs (2 hunks)
  • packages/rs-drive/src/drive/tokens/mint/mod.rs (6 hunks)
  • packages/rs-drive/src/drive/tokens/mint/v0/mod.rs (6 hunks)
  • packages/rs-drive/src/drive/tokens/mint_many/v0/mod.rs (1 hunks)
  • packages/rs-drive/src/drive/tokens/paths.rs (2 hunks)
  • packages/rs-drive/src/drive/tokens/system/add_to_token_total_supply/mod.rs (7 hunks)
  • packages/rs-drive/src/drive/tokens/system/add_to_token_total_supply/v0/mod.rs (5 hunks)
  • packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_claim_transition.rs (2 hunks)
  • packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_mint_transition.rs (1 hunks)
  • packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_claim_transition_action/v0/mod.rs (1 hunks)
  • packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_claim_transition_action/v0/transformer.rs (8 hunks)
  • packages/rs-drive/src/util/batch/drive_op_batch/token.rs (5 hunks)
  • packages/rs-drive/src/util/batch/grovedb_op_batch/mod.rs (0 hunks)
  • packages/rs-platform-version/src/version/drive_versions/drive_token_method_versions/mod.rs (1 hunks)
  • packages/rs-platform-version/src/version/drive_versions/drive_token_method_versions/v1.rs (1 hunks)
  • packages/rs-platform-version/src/version/mocks/v2_test.rs (1 hunks)
  • packages/rs-platform-version/src/version/system_limits/mod.rs (1 hunks)
  • packages/rs-platform-version/src/version/system_limits/v1.rs (1 hunks)
  • packages/wasm-dpp/src/errors/consensus/consensus_error.rs (2 hunks)
💤 Files with no reviewable changes (17)
  • packages/rs-drive/src/drive/tokens/estimated_costs/for_token_perpetual_distribution/mod.rs
  • packages/rs-dpp/src/data_contract/associated_token/token_configuration_localization/mod.rs
  • packages/rs-dpp/src/data_contract/v0/serialization/mod.rs
  • packages/rs-dpp/src/data_contract/v0/methods/schema.rs
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_update/state/v0/mod.rs
  • packages/rs-drive/src/drive/tokens/estimated_costs/for_token_perpetual_distribution/v0/mod.rs
  • packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_type/accessors.rs
  • packages/rs-dpp/src/data_contract/v1/serialization/mod.rs
  • packages/rs-drive/src/util/batch/grovedb_op_batch/mod.rs
  • packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v0/mod.rs
  • packages/rs-dpp/src/data_contract/v1/methods/schema.rs
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual.rs
  • packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/find_oldest_epoch_needing_payment/v0/mod.rs
  • packages/rs-drive/src/drive/contract/insert/insert_contract/v1/mod.rs
  • packages/rs-dpp/src/data_contract/associated_token/token_configuration_convention/accessors/mod.rs
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/state/v0/mod.rs
  • packages/rs-drive/src/drive/tokens/distribution/add_perpetual_distribution/mod.rs
🧰 Additional context used
🧠 Learnings (3)
packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/methods/mod.rs (1)
Learnt from: QuantumExplorer
PR: dashpay/platform#2450
File: packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/v0/methods.rs:10-12
Timestamp: 2025-03-13T13:59:31.360Z
Learning: Block interval calculations in token distribution logic should use checked arithmetic operations (checked_sub, checked_add) to prevent potential overflows, especially when dealing with block heights and intervals.
packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/v0/methods.rs (1)
Learnt from: QuantumExplorer
PR: dashpay/platform#2450
File: packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/v0/methods.rs:10-12
Timestamp: 2025-03-13T13:59:31.360Z
Learning: Block interval calculations in token distribution logic should use checked arithmetic operations (checked_sub, checked_add) to prevent potential overflows, especially when dealing with block heights and intervals.
packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_type/mod.rs (1)
Learnt from: QuantumExplorer
PR: dashpay/platform#2450
File: packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/v0/methods.rs:10-12
Timestamp: 2025-03-13T13:59:31.360Z
Learning: Block interval calculations in token distribution logic should use checked arithmetic operations (checked_sub, checked_add) to prevent potential overflows, especially when dealing with block heights and intervals.
⏰ Context from checks skipped due to timeout of 90000ms (5)
  • GitHub Check: Rust packages (wasm-dpp) / Tests
  • GitHub Check: Rust packages (drive-abci) / Tests
  • GitHub Check: Rust packages (drive-abci) / Linting
  • GitHub Check: Rust packages (dpp) / Check each feature
  • GitHub Check: Build JS packages / Build JS
🔇 Additional comments (245)
packages/rs-drive/src/drive/initialization/v1/mod.rs (2)

7-12: Removal of unused imports improves code cleanliness

The consolidation of imports and removal of unused ones (token_perpetual_distributions_path_vec, token_root_perpetual_distributions_path_vec, BatchInsertTreeApplyType, TreeType) makes the code cleaner and easier to maintain. This change aligns with the PR's focus on token distribution fixes.


18-18: Simplified import statement

The removal of TreeType from this import statement is consistent with the overall cleanup of unused imports.

packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/mod.rs (1)

2-2:

✅ Verification successful

Test coverage expanded with pre-programmed distribution tests

Adding a new test module for pre-programmed distributions aligns with the PR objective of improving token distribution functionality and increasing test coverage.


🏁 Script executed:

#!/bin/bash
# Check that the pre_programmed.rs test file exists and contains tests
fd -t f "pre_programmed.rs" packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/ | xargs cat

Length of output: 29971


Pre-Programmed Distribution Tests Verified

The new test module has been correctly added. The file packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/pre_programmed.rs contains comprehensive tests covering various scenarios for pre-programmed token distributions, including successful claims and handling of cases where no rewards are available. This aligns well with the PR objective of expanding test coverage for token distribution functionality.

packages/rs-drive/src/drive/tokens/distribution/fetch/mod.rs (1)

2-2:

❓ Verification inconclusive

New functionality for tracking pre-programmed distribution payments

This module addition introduces capabilities to track the last payment timestamp for pre-programmed token distributions, which is essential for proper token distribution management.


🏁 Script executed:

#!/bin/bash
# Check the implementation of the new module
fd -t f "pre_programmed_distribution_last_paid_time_ms.rs" packages/rs-drive/src/drive/tokens/distribution/fetch/ | xargs cat

Length of output: 125


Attention: Verify Existence of Implementation for Tracking Module

The new module declaration in packages/rs-drive/src/drive/tokens/distribution/fetch/mod.rs:

mod pre_programmed_distribution_last_paid_time_ms;

appears to be missing its corresponding implementation file (pre_programmed_distribution_last_paid_time_ms.rs). The initial verification command returned no output, which may indicate one of the following:

  • The file is missing from the repository.
  • The file is misnamed or located elsewhere.
  • The implementation is pending, and the module declaration was added prematurely.

Action Required:

  • Manual Verification: Please verify that the file exists, is correctly named, and contains the intended logic to track the last payment timestamp for pre-programmed token distributions.
  • Update if Needed: If the file is missing or mislocated, update the repository accordingly to ensure the new functionality works as expected.
packages/rs-platform-version/src/version/drive_versions/drive_token_method_versions/v1.rs (1)

20-20: Integration of pre-programmed distribution tracking.

A new field pre_programmed_distribution_last_paid_time has been added with an initial version of 0, which aligns with the token distribution fixes mentioned in the PR title. This field will enable tracking the last payment time for pre-programmed distributions, complementing the existing perpetual_distribution_last_paid_time field.

packages/rs-platform-version/src/version/drive_versions/drive_token_method_versions/mod.rs (1)

36-36: New feature field for tracking pre-programmed distribution payment times.

The addition of this field to the DriveTokenFetchMethodVersions struct complements the existing perpetual distribution tracking, completing the API for both distribution types. This change matches the initialization in the v1.rs file.

packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/reward_ratio.rs (1)

1-5: New ratio representation for reward calculations.

The RewardRatio struct provides a clean way to represent reward ratios with numerator and denominator values. This structure is appropriately derived with Debug, Copy, and Clone traits for convenient use in token distribution logic.

packages/rs-dpp/src/data_contract/document_type/property/mod.rs (1)

19-19: Removed unused import for BTreeValueMapPathHelper.

The removal of this import suggests a cleanup or refactoring of the BTree operations. Since the import is no longer needed, this change reduces unnecessary dependencies, which is a good practice.

packages/rs-platform-version/src/version/mocks/v2_test.rs (1)

394-395: Consistent implementation of max token redemption cycles limit.

The added max_token_redemption_cycles field with value 128 aligns with the new system limit definition. This ensures the test platform version correctly represents the new constraint for token redemption cycles.

packages/rs-platform-version/src/version/system_limits/mod.rs (1)

13-17: Well-documented field for redemption cycle limitation.

The addition of max_token_redemption_cycles with clear documentation properly explains the performance considerations behind this limit. The comments effectively communicate why constant distributions are more efficient and why other distribution methods need this limitation.

packages/rs-dpp/src/errors/protocol_error.rs (1)

284-286: Added new error variant for missing epoch information.

The new MissingEpochInfo error variant enhances error reporting capabilities related to epoch data, which aligns with the token distribution fixes in this PR.

packages/rs-drive/src/drive/tokens/mint_many/v0/mod.rs (1)

132-143:

✅ Verification successful

Updated API call to handle saturation control and tuple return.

The modification adds a false parameter for allow_saturation and extracts only the first element of the returned tuple with .0. This change:

  1. Provides explicit control over token supply saturation during minting
  2. Adapts to the updated API that now returns multiple values

Ensure all other callers of this method have been similarly updated to handle the new return type and parameter.


🏁 Script executed:

#!/bin/bash
# Check if other callers of add_to_token_total_supply_operations have been updated

# Search for all calls to this method
echo "Searching for all calls to add_to_token_total_supply_operations:"
rg "add_to_token_total_supply_operations\s*\(" --type rust

Length of output: 534


Verified: All Callers Updated for New API Signature

The changes in the mint-many file correctly add the false parameter and extract the first element from the returned tuple. Our search confirmed that the other call sites in packages/rs-drive/src/drive/tokens/mint/v0/mod.rs and packages/rs-drive/src/drive/tokens/system/add_to_token_total_supply/mod.rs are also updated accordingly. No further modifications are needed.

packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_claim_transition_action/v0/mod.rs (1)

106-106:

✅ Verification successful

Simplified match pattern for TokenDistributionInfo::Perpetual variant.

The match pattern for the TokenDistributionInfo::Perpetual variant has been updated to expect only two parameters rather than three, which aligns with structural changes to the TokenDistributionInfo enum elsewhere in the codebase.


🏁 Script executed:

#!/bin/bash
# Verify that the TokenDistributionInfo::Perpetual variant definition is consistent with this usage

# Look for the TokenDistributionInfo enum definition
rg -A 20 "enum TokenDistributionInfo" --type rust

Length of output: 2685


Approved: Match pattern update for TokenDistributionInfo::Perpetual is correct

  • Verified that the TokenDistributionInfo::Perpetual variant is defined as accepting exactly two parameters (a RewardDistributionMoment and a TokenDistributionResolvedRecipient) in packages/rs-dpp/src/data_contract/associated_token/token_distribution_key.rs.
  • The updated match arm in packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_claim_transition_action/v0/mod.rs correctly deconstructs the variant into (_, resolved_recipient).

The change aligns with the current enum definition, so no further modifications are needed.

packages/wasm-dpp/src/errors/consensus/consensus_error.rs (2)

87-87: Added new token claim error types to imports.

The import statement for the token module has been updated to include the new error types InvalidTokenClaimNoCurrentRewards and InvalidTokenClaimWrongClaimant.

These new error types will improve error handling for token claim operations.


381-386: Added handler cases for new token claim error types.

Two new cases have been added to the from_state_error function to handle the InvalidTokenClaimNoCurrentRewards and InvalidTokenClaimWrongClaimant state errors.

These additions properly integrate the new error types into the existing error handling system.

packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual/mod.rs (1)

1-3: New test modules for perpetual token distribution.

Added new test modules for block-based and time-based perpetual token distribution.

This is a good addition that will improve test coverage for different types of perpetual token distribution mechanisms. The structure with specialized modules for each distribution type (block_based and time_based) follows good organization practices.

packages/rs-dpp/src/block/finalized_epoch_info/v0/mod.rs (1)

8-8: Good change: Vec<(Identifier, u64)> → BTreeMap<Identifier, u64>

Switching from a vector of tuples to a BTreeMap improves both semantics and performance:

  • Provides O(log n) lookups by Identifier (vs O(n) in vector)
  • Maintains ordering based on keys
  • Ensures uniqueness of Identifiers
  • Makes the code more self-documenting

Also applies to: 32-32

packages/rs-dpp/src/block/finalized_epoch_info/getters.rs (1)

6-6: Good change: Updated return type to match FinalizedEpochInfoV0 changes

Correctly updated the return type of block_proposers() from &Vec<(Identifier, u64)> to &BTreeMap<Identifier, u64> to maintain consistency with the struct field type change. This ensures type safety and proper API expectations.

Also applies to: 63-63

packages/rs-dpp/src/block/finalized_epoch_info/v0/getters.rs (1)

5-5: Good change: Updated both trait definition and implementation return types

Properly updated both the trait definition and implementation to return &BTreeMap<Identifier, u64> instead of &Vec<(Identifier, u64)>, maintaining consistency with the struct field type change.

Also applies to: 37-37, 83-83

packages/rs-dpp/src/errors/consensus/state/state_error.rs (1)

245-249: Good error handling expansion

Adding these two specific error variants improves the system's ability to provide clear and precise feedback for token claim operations. The InvalidTokenClaimNoCurrentRewards and InvalidTokenClaimWrongClaimant errors will help developers diagnose claim-related issues more effectively.

Using #[error(transparent)] ensures the underlying error details are properly surfaced, maintaining the error context.

packages/rs-drive/src/drive/tokens/estimated_costs/for_token_pre_programmed_distribution/mod.rs (1)

28-28: Good API improvement making the times parameter optional

Making the times parameter optional is a good improvement to the API. This change allows for more flexibility, as callers can now omit the timestamps when they're not needed, while maintaining backward compatibility with existing code that provides timestamps.

The documentation has been properly updated to reflect this change.

Also applies to: 44-44

packages/rs-dpp/src/data_contract/associated_token/token_distribution_key.rs (2)

51-54: Simplified data model for Perpetual distributions

The Perpetual variant of TokenDistributionInfo has been simplified to contain only one RewardDistributionMoment instead of two. This streamlines the data structure, making it more maintainable and easier to understand.

The documentation comments have been updated appropriately to reflect the change in structure.


63-64: Updated implementations to match the simplified data model

The From trait implementations have been correctly updated to match the simplified Perpetual variant structure, ensuring that the conversion logic remains consistent with the data model changes.

Also applies to: 76-77

packages/rs-dpp/src/errors/consensus/state/token/mod.rs (1)

6-8: Good addition of token claim-specific error modules

The addition of the two new error modules for token claims (invalid_token_claim_no_current_rewards and invalid_token_claim_wrong_claimant) with their corresponding pub use statements enhances the error handling system.

These modules correspond to the new error variants added to the StateError enum, providing a more granular and accurate approach to error handling for token claim operations.

Also applies to: 25-27

packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/methods/mod.rs (3)

5-5: New import for RewardDistributionMoment added

This import is required for the updated return types of the methods below. Good addition.


40-44: Method return type updated to use more descriptive type

The method now returns a RewardDistributionMoment instead of a u64. This enhances type safety and provides better semantic meaning to what was previously a primitive numeric value.


46-50: New method for getting current interval

The addition of the current_interval method provides a complementary function to next_interval, improving the API by allowing clients to determine the current distribution moment.

packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/methods/v0/mod.rs (1)

3-3: New import for RewardDistributionMoment

This import is required for the updated trait method return types.

packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_claim_transition.rs (3)

72-73: Field name standardization in TokenMarkPreProgrammedReleaseAsDistributed

The recipient field has been renamed to recipient_id for more consistent naming across the codebase.


77-85: Updated TokenMarkPerpetualReleaseAsDistributed fields

The fields for the perpetual release distribution have been updated:

  1. Pattern matching now only captures claim_moment
  2. Changed field from owner_id to recipient_id
  3. Added new field cycle_start_moment

These changes improve the tracking of distribution cycles and standardize recipient identification.


62-62:

❓ Verification inconclusive

New parameter for token minting allows saturation

The allow_saturation parameter has been added and set to true. This allows token minting to potentially reach the maximum supply limit. Make sure this is the intended behavior as it could affect token economics.

Run the following script to check if there are tests validating the behavior with allow_saturation set to true:


🏁 Script executed:

#!/bin/bash
# Search for test cases that verify token saturation behavior
rg -A 5 -B 5 "allow_saturation.*true" --type rust | grep -E "test|assert"

Length of output: 75


Action Required: Verify Token Saturation Behavior in Tests

It appears that automated searching for tests explicitly validating the behavior of allow_saturation: true did not yield any results. This raises a concern regarding whether the token minting behavior—allowing saturation to possibly reach the maximum supply limit—is properly covered by the test suite. Please manually verify that:

  • Appropriate test cases exist which validate the behavior when allow_saturation is set to true.
  • The current tests (if any, or tests that cover related functionality) robustly ensure that the token economics are maintained as intended.
  • If such tests are missing, consider adding tests to explicitly cover this scenario.

File: packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_claim_transition.rs (line 62)
Snippet:

                            allow_saturation: true,
packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/encode.rs (4)

44-46: Improved variable naming in Linear distribution function

Renaming s to start_step and b to starting_amount improves code readability by using descriptive names that clearly indicate the purpose of these variables.


63-64: More descriptive variable naming in mathematical distribution functions

Renaming s to start_moment in the Polynomial, Exponential, Logarithmic, and InvertedLogarithmic variants improves code clarity by using a descriptive name that clearly indicates the purpose of this variable.

Also applies to: 85-86, 107-108, 129-130


195-197: Consistent naming in Decode implementation

The variable naming updates have been consistently applied in the Decode implementation, ensuring that the code remains coherent throughout the encoding and decoding processes.

Also applies to: 217-218, 239-240, 261-262, 283-284


341-343: Consistent naming in BorrowDecode implementation

The variable naming updates have been consistently applied in the BorrowDecode implementation, maintaining coherence throughout all serialization and deserialization code paths.

Also applies to: 363-364, 385-386, 407-408, 429-430

packages/rs-drive/src/drive/tokens/distribution/fetch/pre_programmed_distribution_last_paid_time_ms/v0/mod.rs (2)

53-62: Decode logic is correct and carefully validates the 8-byte length.

This section properly checks that the stored value has a length of 8 bytes and then converts it to a u64 via from_be_bytes. This ensures well-defined behavior and avoids accidentally decoding corrupted or malformed data.


64-71: Thoughtful handling of missing or invalid items.

Returning Ok(None) when the key is not found simplifies usage for callers by uniformly signaling “no record found.” Handling unexpected item types and other GroveDB errors by returning appropriate Drive errors covers all edge cases and reduces the chance of silent failures.

packages/rs-drive/src/drive/tokens/distribution/mark_pre_programmed_release_as_distributed/mod.rs (1)

57-65: LGTM - Function call updated consistently

The function call to the versioned implementation has been correctly updated to pass recipient_id as the parameter.

packages/rs-drive/src/drive/tokens/estimated_costs/for_token_pre_programmed_distribution/v0/mod.rs (2)

62-64: Good defensive coding with fallback value

The code now gracefully handles the case when times is None by providing a default value of 128 for the estimated layer count.


69-83: LGTM - Conditional wrapping prevents potential runtime errors

The loop for processing each timestamp is now properly wrapped in a conditional to execute only when times is Some. This prevents potential runtime errors when times is None.

packages/rs-drive/src/util/batch/drive_op_batch/token.rs (6)

41-43: Well-documented saturation parameter

Good job adding explanatory comments for the new allow_saturation parameter that clarifies its purpose and provides an example.


58-65: Improved token release distribution with better documentation

The TokenMarkPerpetualReleaseAsDistributed variant has been improved by:

  1. Replacing multiple fields with the clearer recipient_id
  2. Adding the cycle_start_moment field
  3. Including detailed documentation for the new fields

This change makes the API more intuitive and better documented.


69-73: Consistent parameter naming with TokenMarkPerpetualReleaseAsDistributed

The TokenMarkPreProgrammedReleaseAsDistributed variant now uses consistent parameter naming with the perpetual release variant, using recipient_id instead of separate owner and identity identifiers.


155-156: Saturation parameter correctly propagated

The new allow_saturation parameter is correctly propagated from the enum variant to the drive function call.

Also applies to: 164-165


262-275: Implementation correctly updated for TokenMarkPerpetualReleaseAsDistributed

The implementation for TokenMarkPerpetualReleaseAsDistributed has been correctly updated to use the new parameters.


276-292: Implementation correctly updated for TokenMarkPreProgrammedReleaseAsDistributed

The implementation for TokenMarkPreProgrammedReleaseAsDistributed has been correctly updated to use the new parameters.

packages/rs-drive/src/drive/tokens/distribution/add_perpetual_distribution/v0/mod.rs (2)

2-5: New token distribution constants imported

The code now imports important distribution-related constants that will be used later in the function.


80-88:

✅ Verification successful

Storage structure updated for identity tracking

The function now creates an empty tree with TOKEN_PERPETUAL_DISTRIBUTIONS_FOR_IDENTITIES_LAST_CLAIM_KEY instead of the previous handling based on block info and intervals. This change appears to be part of a simplified approach to tracking distribution claims per identity.

Let's check if this constant is properly defined:


🏁 Script executed:

#!/bin/bash
# Look for the definition of TOKEN_PERPETUAL_DISTRIBUTIONS_FOR_IDENTITIES_LAST_CLAIM_KEY
rg -A 1 "TOKEN_PERPETUAL_DISTRIBUTIONS_FOR_IDENTITIES_LAST_CLAIM_KEY" --type rust

Length of output: 2774


Token Constant Verified: Storage Structure Consistency Confirmed

The constant TOKEN_PERPETUAL_DISTRIBUTIONS_FOR_IDENTITIES_LAST_CLAIM_KEY is defined correctly in packages/rs-drive/src/drive/tokens/paths.rs (set to 192) and is consistently referenced across the related modules. The changes reflecting a simplified approach to tracking distribution claims per identity using an empty tree are verified to rely on the proper constant definition.

  • Verified definition in packages/rs-drive/src/drive/tokens/paths.rs
  • Consistent usage in modules such as add_perpetual_distribution and mark_perpetual_release_as_distributed

No issues remain regarding this change.

packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/v0/methods.rs (2)

3-4: Good modularization of the reward distribution framework

These imports show a clear separation of reward distribution moment types, which makes the code more maintainable and extensible.


29-47: The current_interval method includes an important edge case for epoch interval

The method now correctly handles the special case when interval equals 1, directly returning the current epoch index. This is an important edge case that prevents unnecessary calculations.

The return types have been properly changed to the appropriate variant of RewardDistributionMoment, aligning with the refactoring of the next_interval method and providing consistency across the API.

packages/rs-dpp/src/errors/consensus/state/token/invalid_token_claim_wrong_claimant.rs (4)

1-8: Appropriate imports for the new error type

All necessary imports for error handling, serialization/deserialization, and protocol structures are included.


9-23: Well-structured error type with clear error message

The error type is well-designed with:

  • Appropriate trait derivations for error handling, debugging, and serialization
  • A descriptive error message that clearly indicates what went wrong
  • Proper fields to hold all relevant identifiers

The unversioned annotation for platform serialization indicates this error structure won't need versioning.


25-53: Complete implementation with constructor and accessors

The implementation provides a constructor and accessors for all fields, following Rust best practices. Each method is properly documented with clear comments explaining its purpose.


55-60: Proper integration with the error hierarchy

The implementation of From<InvalidTokenClaimWrongClaimant> for ConsensusError ensures this error type is properly integrated into the existing error handling infrastructure, allowing it to propagate through the system.

packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_type/evaluate_interval.rs (3)

3-4: Added imports for the enhanced reward calculation logic

The addition of imports for EpochIndex and RewardRatio reflects the expanded functionality to handle epoch-based rewards.


13-16: Updated documentation with improved clarity

The documentation has been enhanced to more accurately describe the function's behavior, particularly regarding distribution start and end moments. The parameter documentation now correctly reflects the renamed parameters and new functionality.

Also applies to: 20-25


31-47: Significantly improved function signature and implementation

The method has been enhanced in several ways:

  1. Added generics to support a flexible callback for epoch reward ratios
  2. Renamed parameters for better clarity
  3. Added a separate parameter for the distribution start moment
  4. Switched to calling evaluate_interval instead of evaluate_interval_in_bounds

These changes make the function more flexible and better structured for different distribution scenarios.

packages/rs-drive/src/drive/tokens/distribution/fetch/pre_programmed_distribution_last_paid_time_ms/mod.rs (3)

1-11: Well-organized imports and module structure

The module structure and imports are clean and well-organized, including only necessary dependencies for the functionality.


12-44: Clear public API method with comprehensive documentation

The fetch_pre_programmed_distribution_last_paid_time_ms method provides a clean public API with:

  • Detailed documentation explaining its purpose and behavior
  • Clear parameter descriptions
  • Proper return type handling with Option<TimestampMillis>
  • Delegation to the operations variant for actual implementation

This follows the pattern of separating the public API from the operations tracking logic.


45-91: Robust versioned implementation with proper error handling

The operations method:

  1. Provides thorough documentation
  2. Handles versioning through the platform version parameter
  3. Implements proper error handling for unknown versions
  4. Follows the repository's pattern for versioned method dispatch

The error message for unknown versions is specific and includes the method name and known versions, which will be helpful for debugging.

packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/evaluate.rs (57)

15-19: Addition of contract_registration_step looks good.
This parameter enables a more precise reference point for distribution. Ensure that any calling code is updated accordingly.


64-64: Good use of unwrap_or(contract_registration_step).
This fallback ensures logic remains consistent if s is absent.


178-178: Renaming the start parameter to start_moment improves clarity.
This name is more expressive and helps prevent confusion about whether it refers to time or an index.


193-193: Consistent fallback to contract_registration_step.
Matches the approach used in other distribution variants.


246-246: Use of start_moment in the exponential distribution is consistent.
Maintains parallel naming across all distribution types.


261-261: Extracted s_val from start_moment is consistent with other code paths.
This keeps logic uniform across multiple distribution branches.


314-314: Implementation mirrors other distribution functions.
Keeping identical naming avoids confusion between distribution variants.


327-327: s_val fallback is coherent.
No issues found; ensures the log-based function has a sane initial offset.


376-376: New start_moment field for inverted logarithmic distribution.
Maintains consistency with the other variants.


399-399: Fallback to contract_registration_step for inverted logarithmic.
Appropriate for ensuring a uniform baseline across distributions.


480-482: Tests for test_fixed_amount are comprehensive.
They validate multiple periods to confirm the function is constant.


493-499: Stepwise emission tests offer good coverage.
Verifies boundary periods (both ends of step ranges).


512-518: Checks on step decreasing function are thorough.
The test ensures the function doesn’t go below the min_value.


532-534: Proper handling of a divide-by-zero scenario.
The test ensures the function gracefully fails when the denominator is 0.


544-545: Random distribution test with range check.
Confirms that values stay within the specified minimum and maximum.


559-560: Single-value random distribution test is valid.
Ensures the function behaves deterministically if min == max.


572-577: Error path for invalid range is verified.
Correctly expects an Overflow error when min > max.


584-586: Ensures deterministic output for same seed x.
Important property for verifying predictable random generation.


597-599: Test confirms variation across different x.
Verifies that changing seed yields different outputs.


613-623: Linear function increasing test is well-scoped.
Covers both the base offset and additive slope.


636-639: Linear function decreasing test.
Ensures correct floor clamp to min_value.


653-655: Divide-by-zero for linear function is properly tested.
Verifies the function rejects d=0.


674-678: Polynomial function test checks multiple x values.
Good coverage for verifying exponent and offset logic.


694-700: Overflow test for large coefficient.
Ensures the function fails gracefully if results exceed numeric limits.


717-718: Fractional exponent in polynomial function is validated.
Confirms (3/2) exponent yields correct results.


735-736: Test for negative a verifies non-negative result clamp.
Guarantees negative outcomes become zero.


754-757: Shift parameter start_moment is tested.
Ensures polynomial function uses shifted domain.


775-776: Offset parameter o tested.
Checks whether the function applies offset before exponentiation.


794-795: Constant polynomial function scenario is confirmed.
(x - s + o)^0 = 1 for valid domain, ensuring we get a flat outcome.


813-814: Linear-like polynomial test (m=1, n=1).
Proper coverage ensures f(x)=ax+b works within polynomial logic.


831-832: Cubic function test.
Verifies x^3 usage and correct final values.


850-851: Combined offset and shift for polynomial.
Ensures the function handles multiple transformations.


869-871: Basic exponential function.
Checks both base offset (c) and exponent correctness.


887-889: Divide-by-zero for exponential function.
Test ensures an error is thrown when d = 0.


907-909: Exponential function with small x coverage.
Verifies results for incremental x changes and bases.


926-929: Slow-growth exponential test.
Confirms that smaller exponents produce smaller increments over large x.


945-950: Rapid-growth exponential is clamped.
Tests upper bound to verify no overflow beyond max_value.


966-969: Exponential with negative exponent saturating to a minimum.
Verifies results never drop below 10 if min_value is set.


985-988: Exponential function with custom min_value.
Ensures the result never falls below 11.


1005-1014: Start at max scenario test.
Guaranteed clamping for initial call ensures no overshoot of the max bound.


1030-1036: Overflow test for huge x.
Properly rejects extremely large arguments that exceed numeric bounds.


1054-1056: Logarithmic function basic test.
Checks initial offset to ensure valid log domain.


1072-1074: Enforces min_value and max_value for logarithmic.
Ensures result is correctly clamped.


1091-1093: Handles undefined log domain.
Tests a scenario with log(0) to ensure overflow is reported.


1110-1113: Large x scenario for log function.
Checks no overflow occurs and that results remain finite.


1130-1132: Divide-by-zero in d for log function.
Asserts the function halts gracefully when d=0.


1150-1152: Divide-by-zero in n for log function.
Prevents invalid exponent denominator usage.


1171-1173: Inverted log decreasing test.
Checks that consecutive blocks yield decreasing amounts.


1190-1193: Inverted log increasing test.
Confirms correct growth when a is negative and n is large, reversing the slope.


1212-1213: Clamp to 0 if final amount is negative.
The test ensures negative outcomes become zero.


1229-1230: Clamps result to min_value for negative outcomes.
Verifies distribution won’t drop below set minimum.


1247-1248: Test ensures max_value is enforced.
Keeps the result within an upper bound for large negative a.


1265-1267: Ensures log argument isn’t zero or negative.
Catches an overflow error if o causes (x - s + o) to be 0.


1285-1287: Zero n in inverted log triggers divide-by-zero error.
Protects from invalid domain.


1305-1307: Zero d in inverted log triggers divide-by-zero error.
Ensures no attempt to divide by zero occurs.


1325-1328: min_value clamp for increasing inverted log distribution.
The function’s initial result starts at min_value and ends at max_value.


1351-1354: Final clamp to min_value in a decreasing distribution.
Prevents negative or smaller-than-min outcomes.

packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_claim_transition_action/v0/transformer.rs (16)

5-5: Explicit import of TokenAmount is clear.
Ensures the balance type is well-defined.


9-9: Use of FinalizedEpochInfoGettersV0 is consistent.
Allows retrieving epoch details more easily.


12-12: Introduction of new errors: InvalidTokenClaimNoCurrentRewards, InvalidTokenClaimWrongClaimant.
These specific errors improve clarity and user feedback.


17-17: RewardRatio usage for distributing partial tokens is good.
Confirms more flexible calculations for distribution events.


19-19: Leveraging method trait from TokenPerpetualDistributionV0Methods.
This helps unify distribution logic.


21-21: TokenPreProgrammedDistributionV0Methods import
Allows direct usage of pre-programmed distribution logic.


29-29: Combined usage of ConsensusValidationResult and UserFeeIncrease.
Streamlines how we pass fee adjustments and consensus checks.


225-307: Retrieval of the oldest unclaimed distribution is correct.

  1. Filters future timestamps and uses the earliest unclaimed for the identity.
  2. Properly raises InvalidTokenClaimNoCurrentRewards when none are available.

309-312: Well-defined final output for pre-programmed scenario.
TokenDistributionInfo::PreProgrammed captures the distribution type and moment.


344-383: Claimant verification logic in perpetual distribution.

  1. Ensures only the designated identity can claim.
  2. Raises InvalidTokenClaimWrongClaimant with explicit info.

410-411: Obtaining contract creation moment from distribution_type.
Helps compare it against the last-paid moment for correct intervals.


423-424: current_interval usage for block-based or time-based distribution.
Matches the updated logic in distribution crates.


426-433: Compute the maximum cycle to avoid indefinite claims.
Leverages a platform limit (max_token_redemption_cycles).


435-447: Accumulative reward calculation via rewards_in_interval.
Correctly sums up partial or total rewards across epochs.


504-530: Handling zero-amount claims with InvalidTokenClaimNoCurrentRewards.
Prevents repeated claims of zero tokens.


534-535: Return of (amount, TokenDistributionInfo::Perpetual(...)).
Captures the needed distribution variant and moment.

packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual/block_based.rs (32)

1-4: Import of test helpers and standard crates.
Provides a well-structured base for setting up platform states and identities.


5-9: Successfully references TokenConfiguration and BatchTransition.
Makes state transitions straightforward to construct and serialize.


10-11: Usage of rand::prelude::StdRng
Ensures reproducible random data for test consistency.


12-19: Definition of mod perpetual_distribution_block.
Logical grouping for block-based distribution tests.


22-28: Sets up platform with the latest protocol version.
Prepares a stable environment for block-based distribution tests.


29-30: Random seed usage enhances reproducibility.
Prevents flakiness in tests that rely on random identity creation.


33-35: Generates first identity for test.
Allocates 0.5 Dash in credits for immediate contract usage.


36-55: Creates and configures a token contract with a fixed 50-credit emission.
Implementing block-based intervals ensures repeated reward cycles.


57-57: fast_forward_to_block(..., 40, ...) simulates block progression.
Effectively checks reward accrual over a large skip.


59-75: Creates a token claim transition
Ensures usage of the new perpetual distribution claim logic with block-based triggers.


77-79: Serializes the batch transition for processing.
Confirms that raw transitions can be submitted to the platform.


81-82: Transaction context usage.
Starts a fresh transaction in Grove for each test scenario.


83-99: Processes the raw transition
Ensures the platform can parse and validate the claim.


101-104: Asserts successful state transition execution.
Verifies no errors arise when the claim is valid and rewards exist.


106-111: Commits transaction
Saves the state to finalize the distribution claim.


113-124: Fetches and confirms token balance
Validates that the user’s account is credited with the correct reward.


125-125: Fast-forwards to a new block
Sets up a second claim under different height/time conditions.


127-143: Second claim transition
Tests the scenario where no new rewards were generated, expecting an error.


145-147: Re-serialization for the second claim
Proves transitions remain consistent across repeated requests.


149-166: Platform processes second claim
Verifies that the engine detects “no new rewards” and throws an error.


169-175: Expects InvalidTokenClaimNoCurrentRewards consensus error.
Accurately reflects the scenario of zero additional distribution.


177-183: Commits state for the second transition
Even though it failed, the transaction finalizes updated state or logs.


184-194: Balance check
Confirms that unsuccessful claims do not alter existing token holdings.


195-195: Fast-forward again
Simulates additional blocks for another claim attempt.


197-213: Third claim attempt
Ensures that newly created intervals produce a successful claim.


215-218: Serialization check
Same process repeated to confirm stable handling of transitions.


219-236: Process the third claim
Expects success due to newly matured rewards.


239-242: Asserts a successful result
Indicates that the periodic logic properly recognized new rewards.


244-250: Final commit
Writes the newly updated token balance after the successful claim.


251-262: Confirms final token balance increment
Demonstrates that an extra 50 tokens have been added.


265-386: test_token_perpetual_distribution_not_claimant

  1. Validates that only the specified recipient can claim token rewards.
  2. Ensures the error matches InvalidTokenClaimWrongClaimant.

388-495: test_token_perpetual_distribution_block_claim_linear_given_to_specific_identity

  1. Ensures that distribution is properly assigned to a single non-owner identity.
  2. The final balance check confirms correct accumulation.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/mod.rs (3)

59-59: Added TimestampMillis import for improved contract time handling.

This addition is part of enhancing the contract creation functionality to support customizable timestamps.


2295-2295: Added contract_start_time parameter for flexible contract creation.

This parameter allows for specifying when a contract starts, improving the flexibility of contract creation by allowing timestamps other than the default value.


2308-2308: Improved contract creation time handling.

The code now uses the provided start time or defaults to 0 instead of hardcoding the value, which aligns with the function signature change.

packages/rs-dpp/src/errors/consensus/codes.rs (2)

236-237: Expanded document errors range for better organization.

The document errors range has been expanded from 40100-40149 to 40100-40199, providing more space for document-related error codes.


288-307: Added new token error codes for improved error handling.

A new range for token-specific errors has been introduced (40700-40799), including specialized errors for token claims. This improves error handling granularity for token operations.

Key additions include:

  • 40716: InvalidTokenClaimNoCurrentRewards
  • 40717: InvalidTokenClaimWrongClaimant

These new error types support the token distribution fixes mentioned in the PR title.

packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/add_epoch_pool_to_proposers_payout_operations/v0/mod.rs (3)

3-3: Added BTreeMap import for improved proposer data structure.

This import supports the change from vector to BTreeMap for proposers' data.


36-36: Changed return type to use BTreeMap instead of vector for proposers.

Using BTreeMap instead of a vector of tuples provides more efficient lookup by Identifier and maintains a consistent order of proposers.


169-169: Updated return statement to convert proposers to BTreeMap.

The implementation now correctly converts the vector of tuples into a BTreeMap to align with the return type change.

packages/rs-drive/src/drive/tokens/distribution/mark_perpetual_release_as_distributed/mod.rs (6)

14-15: Improved function description for better clarity.

The documentation now correctly describes the function as marking a token distribution as distributed rather than a token release.


16-22: Enhanced documentation with clearer explanation of function behavior.

The updated documentation provides more details about what actions the function performs, making it easier to understand its purpose and effects.


24-29: Updated parameter descriptions for better API documentation.

The parameter descriptions are now more precise, particularly clarifying the purpose of recipient_id and current_moment.


31-33: Updated return type documentation.

The return type has been changed to more accurately reflect that the function returns batch operations for storage updates.


35-35: Simplified versioning documentation.

The versioning section has been streamlined to be more concise while still providing the necessary information.


39-40: Simplified parameter list for token distribution.

Parameters have been refactored from owner_id, previous_moment, and next_moment to the more appropriate recipient_id and current_moment. This change streamlines the function and makes its purpose clearer.

packages/rs-drive/src/drive/tokens/paths.rs (3)

62-64: Watch out for potential key collisions.

Both TOKEN_PERPETUAL_DISTRIBUTIONS_FOR_IDENTITIES_LAST_CLAIM_KEY and TOKEN_PRE_PROGRAMMED_DISTRIBUTIONS_FOR_IDENTITIES_LAST_CLAIM_KEY share the same value (192). This may lead to collisions in certain contexts if they are not distinctly separated in their respective path structures. Consider assigning a unique value if these constants are meant to be logically separate.


230-241: Maintain naming consistency.

The _vec version precisely mirrors the _path version, which is good. Ensure both functions remain in sync if future parameters or logic are introduced or changed.


243-271: Validate usage of extended identity path.

The new functions token_perpetual_distributions_identity_last_claimed_by_identity_path/_vec expand the path to six segments. Confirm that all relevant code properly handles this deeper path to avoid indexing mistakes or mismatches.

packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/pre_programmed.rs (3)

188-359: Validate progression of block time for subsequent claims.

The second test (test_token_pre_programmed_distribution_claim_again_when_none_left) successfully checks the scenario of claiming after rewards are exhausted. It might be beneficial to verify edge cases such as extremely close subsequent blocks or missing time increments.


361-642: Solid coverage for delayed distributions.

test_token_pre_programmed_distribution_claim_again_when_none_ready comprehensively tests deferred rewards. The approach looks correct and ensures error handling is robust for scenarios where the claim is too early.


644-748: Good negative test coverage for impossible claims.

test_token_pre_programmed_distribution_claim_no_rewards_for_recipient accurately demonstrates the scenario where rewards were never allocated to the claimant. This helps ensure that the system properly rejects such claims.

packages/rs-drive/src/drive/tokens/distribution/mark_pre_programmed_release_as_distributed/v0/mod.rs (4)

2-5: No apparent issues with new imports.

The newly added imports appear relevant to the updated logic. There are no obvious duplications or unused references, so this looks fine.

Also applies to: 13-13, 15-15, 26-29


58-58: Parameter rename to recipient_id is clear.

Replacing owner_id/identity_id with recipient_id streamlines the API and better describes its use. This improves clarity for the caller.


68-69: Good addition of estimation logic.

The inserted estimation logic for pre_programmed_distributions_path helps ensure that storage costs are accurately calculated. The approach of updating estimated_costs_only_with_layer_info with relevant layer sizes appears consistent with existing patterns.

Also applies to: 72-73, 81-89, 96-103


162-170: Verify that new insert key won't overwrite multiple scheduled distributions.

Storing release_time as the value under recipient_id might cause overwrites if there are multiple releases for the same recipient. Confirm whether only one claim is intended per identity at a time or if additional key segmentation is necessary.

packages/rs-drive/src/drive/tokens/distribution/mark_perpetual_release_as_distributed/v0/mod.rs (7)

1-1: Good usage of tokens::paths.
No concerns, as it ensures correct referencing of distribution paths.


8-11: Imports for Grovedb cost estimation are properly introduced.
These newly added imports perfectly align with the batch operation usage and cost annotation needs.


19-20: Function signature update to recipient_id and current_moment.
Switching from the old parameters (owner_id, previous/next moment) to these streamlined arguments looks consistent with the updated distribution approach.


28-30: Revised path usage for perpetual distributions.
Using token_perpetual_distributions_identity_last_claimed_time_path_vec to define the storage path neatly aligns with the focus on the current moment.


38-42: Unclear rationale for epoch-based estimated layer depth.
Currently set to EstimatedLevel(10, false) for epoch-based distributions but EstimatedLevel(0, false) for block/time-based. Ensure this difference is intentional and accurately reflects domain requirements.


44-52: Insertion of estimated layer info for perpetual distributions.
The usage of AllItems(1, 8, None) in EstimatedLayerInformation follows the new approach for storing moment data. No issues identified here.


58-58: Storing the distribution moment as a raw item.
Using current_moment.to_be_bytes_vec() is a clean, type-safe way of persisting the numeric representation.

packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_type/mod.rs (3)

5-5: New imports for interval-based distributions.
Bringing in block/time/epoch intervals is consistent with the revised approach to reward distribution.


15-36: Refined documentation on block/time/epoch-based distribution variants.
The clarifications help show exactly when distributions begin and how they're triggered.


197-197: Updated fmt::Display arms for distributions.
Removing references to start and end keeps the display consistent with the simplified variants.

Also applies to: 205-205, 213-213

packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_moment/mod.rs (8)

6-6: Added Add and Div traits in the standard library import.
Facilitates new arithmetic operations on reward moments.


44-106: Introduced cycle_start method.
Smartly snaps the moment to the greatest multiple of the step by using modulo and saturating subtraction, while handling zero-step as an error.


108-173: steps_till logic for block/time-based calculations.
The inclusive/exclusive boundary flags appear correctly applied, and the zero-step case properly triggers an error.


175-206: Epoch-based steps_till.
This matches the block/time-based approach, ensuring consistent calculation while avoiding overflow via saturating arithmetic.


208-210: Fallback for mismatched moment types.
Returning AddingDifferentTypes error is a clear safeguard against invalid type usage.


222-251: Implementation of Add<u64> for distribution moments.
checked_add ensures that arithmetic overflow surfaces as an explicit error rather than silently wrapping.


286-315: Implementation of Div<u64>.
Rejecting division by zero with a specific error is essential for robust numeric operations.


317-359: impl Div for RewardDistributionMoment.
Good handling of zero divisors via a DivideByZero error. Mismatched types also raise a clear error.

packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/distribution/perpetual/time_based.rs (7)

22-264: Well-structured test scenario.
These lines thoroughly test time-based distribution with incremental claims and ensure correct detection of no-current-rewards after a maximum claim. The logic, especially the usage of fast_forward_to_block and subsequent claims, looks robust.


267-389: Test coverage for invalid claimant scenario.
Verifying that the transition rejects a claimant who is not the designated recipient is vital. These lines effectively ensure the error path is handled, demonstrating correct usage of the new InvalidTokenClaimWrongClaimant error variant.


392-501: Accurate validation of specific identity claim logic.
These lines correctly confirm the correct recipient identity receives token rewards. The approach to verifying the final token balance and time-based intervals is clear and consistent.


693-883: Effective coverage of immediate re-claim error handling.
The test ensures no new rewards are distributed if the claim occurs too soon. This correctly covers negative path validation. No additional concerns here.


886-1074: Precise logic for delayed contract start.
These lines properly test a contract start offset (line 928) by fast-forwarding and verifying partial distribution. The test name clearly indicates the intent. Good job verifying time-based partial accrual.


1077-1258: Stress test for high-slope and starting amount.
This scenario checks large linear slopes and maximum distribution parameters, ensuring the distribution calculation does not overflow for typical usage. Great for boundary testing.


1261-1380: Robust loop-based overflow test.
Iterating the claim operation (lines 1312-1367) methodically validates incremental saturation or overflow behavior. This is a strong coverage approach for stress testing.

packages/rs-drive/src/drive/tokens/mint/mod.rs (2)

54-67: Consider cross-check tests for saturation logic.
Adding calls to token_mint_add_to_operations with allow_saturation toggled ON and OFF in your test suite ensures that the minting process is validated under both scenarios.

Would you like a script to search existing tests in the codebase to verify coverage of these new parameters?


87-100: Maintain consistency in method parameters.
All three public methods (token_mint, token_mint_add_to_operations, token_mint_operations) now incorporate allow_saturation. This is consistent, but ensure any future expansions (e.g., alternative distribution strategies) share a similar approach for clarity.

packages/rs-dpp/src/errors/consensus/state/token/invalid_token_claim_no_current_rewards.rs (2)

10-41: Excellent clarity in error struct.
The struct includes comprehensive fields and getters, conveying enough context to debug incomplete claims. The user-friendly display in #[error(...)] also improves troubleshootability.


72-78: Well-defined conversion into ConsensusError.
Mapping this error variant into a StateError is neatly contained, aligning with the established consensus error patterns.

packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/mod.rs (3)

56-63: Standardization of Explicit None Parameters in Contract Creation Calls

The diff adds an explicit None argument to calls of
create_token_contract_with_owner_identity
across many test scenarios (token minting, token burning, token transfer, token configuration update, token freeze, etc.). This change aligns the tests with the updated API signature by standardizing the parameter list and improving clarity. Please verify that these inserted None values correctly represent the intended default behavior for the newly required parameter.

Also applies to: 144-153, 241-247, 3285-3291, 2969-2976, 3714-3717, 4388-4391, 4857-4860, 5159-5173


4925-4930: Consistent Update in Token Configuration Update Tests

In a couple of token configuration update tests (both in non-group and group scenarios), an explicit None argument was also inserted in the contract–creation call. This adjustment ensures that the setup now conforms to the revised function signature. No logical changes have been introduced; the update is purely for consistency. Please double-check that the parameter position really represents the “unused” or default callback.

Also applies to: 4953-4955


1-6133: Overall: Consistent Signature Adjustments Without Business Logic Changes

Across the entire file, the only modifications consist of inserting explicit None values into function calls (e.g. in the contract creation and configuration transition functions) to match updated signatures. There is no alteration to test logic or control flow. This update improves clarity and ensures consistency across the platform’s token distribution tests. Please ensure that any accompanying documentation or developer notes are updated to reflect the new API requirements.

packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/evaluate_interval.rs (5)

10-27: Nicely documented interval evaluation.

The expanded doc comments clearly explain the function behavior, parameters, and potential error cases. Including the optional get_epoch_reward_ratio parameter highlights flexibility in epoch-based reward calculations. This improvement aids maintainability and readability.


50-50: Good defensive checks on step and interval bounds.

Returning errors (or zero emission) when step == 0 or interval_start_excluded >= interval_end_included is appropriate for preventing invalid intervals. This avoids silent failures or undefined behavior in token emission calculations.

Also applies to: 54-54


58-68: Efficient early return for fixed amounts.

Short-circuiting for DistributionFunction::FixedAmount avoids unnecessary iteration. Calculating steps_count up front is both performant and readable.


88-118: Robust epoch-based ratio handling.

The block applying get_ratio_fn carefully checks for missing epoch info and gracefully handles overflow via checked_mul/checked_div. This reduces the risk of unexpected panics. Test coverage should confirm that partial ratio data triggers the intended error path.

Would you like a test snippet covering scenario(s) with a missing epoch ratio to confirm correct error handling?


28-38:

✅ Verification successful

Validate usage across callers.

evaluate_interval<F> now receives a generic F for epoch-based ratio logic, which is flexible. Ensure that all call sites passing in get_epoch_reward_ratio align with the new type signature. Where it’s unused, consider passing None.


🏁 Script executed:

#!/bin/bash
# Verifies usage of `evaluate_interval` across the codebase
rg -A 10 "evaluate_interval" 

Length of output: 9960


Review Verification: Evaluate Interval Call Sites

After verifying the call sites, all usages of evaluate_interval are aligned with the updated generic signature. In particular, the caller in
packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/reward_distribution_type/evaluate_interval.rs
passes get_epoch_reward_ratio correctly. If epoch-based reward ratio logic isn’t needed at a specific call site, consider explicitly passing None for clarity.

packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/mod.rs (2)

14-14: Added constant improves clarity.

MAX_LINEAR_SLOPE_PARAM being explicitly set to 256 centralizes a key bound. Ensure any validation logic or usage references this constant to avoid magic numbers.


132-132:

✅ Verification successful

Consistent renaming enhances parameter clarity.

Updating fields from s to start_moment (and similarly to start_step or starting_amount) clarifies intent for each distribution variant. This consistency makes the code more approachable. Verify all references to the old fields have been updated throughout the code.

Also applies to: 239-240, 324-325, 382-383, 452-453, 511-511, 566-568, 592-593, 618-619, 644-645, 670-671


🏁 Script executed:

#!/bin/bash
# Confirm no lingering references to old field names "s" or "b"
rg '(\.s\b|\.b\b|s:|b:)'

Length of output: 66200


Renaming Verification: Code Consistency Confirmed

The update from shorthand field names (e.g., s and b) to more descriptive identifiers such as start_moment, start_step, and starting_amount has been applied consistently in the code. A repository-wide grep for the old identifiers produced no unwanted matches in the source code, confirming that all references have been updated.

  • File Verified: packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/mod.rs (and related distribution variant lines)
  • Grep Result: No lingering references to .s, .b, s:, or b: were found in the relevant code files.

The renaming not only clarifies the purpose of each parameter but also improves the overall maintainability of the code.

packages/rs-drive/src/drive/tokens/mint/v0/mod.rs (2)

17-17: New parameter improves overflow control.

Introducing allow_saturation adds an explicit mechanism to handle token supply saturation scenarios. Validate this parameter at call sites to ensure correct usage when near or at total supply limits.

Also applies to: 30-30, 90-90


99-108: Refined logic for actual issuance amount.

Capturing (add_to_supply_operations, actual_issuance_amount) and then applying actual_issuance_amount to the identity’s balance is a robust approach. This ensures token supply changes remain consistent with the newly computed issuance. Comprehensive tests should confirm partial issuance behavior when saturation blocks full issuance.

Would you like a test scenario demonstrating partial issuance under max supply to verify correct behavior?

Also applies to: 110-110, 116-116, 122-122

packages/rs-drive/src/drive/tokens/system/add_to_token_total_supply/mod.rs (5)

7-7: No concerns with the new TokenAmount import.
Replaces a primitive type with a domain-specific TokenAmount, which should improve type safety.


74-74: Good concurrency of parameters.
Forwarding the allow_saturation parameter here maintains consistency across functions.


94-96: Check for potential integer size mismatch with TokenAmount.
Now returning (Vec<LowLevelDriveOperation>, TokenAmount). Confirm the rest of the pipeline expects TokenAmount rather than raw integers and that no unexpected overflow or conversion occurs within the new return type usage.

Also applies to: 102-102


114-114: Consistent call signature propagation.
Forwarding allow_saturation from this method to the v0 layer is cohesive with the rest of the code changes.


37-39:

❓ Verification inconclusive

Double-check call site coverage.
You're forwarding allow_saturation to add_to_token_total_supply_v0. Verify that all external calls properly handle or propagate this bool. This ensures consistent logic across the codebase.


🏁 Script executed:

#!/bin/bash
# Searching for usage of `add_to_token_total_supply` across the repository to confirm correct parameter usage
rg "add_to_token_total_supply\(" -A 4 -B 4

Length of output: 947


Please verify that all higher-level calls propagate allow_saturation correctly.

The grep search only returned the definition in
packages/rs-drive/src/drive/tokens/system/add_to_token_total_supply/mod.rs – we couldn’t identify additional external call sites from this search. This leaves some uncertainty as to whether every caller (or wrapper) that invokes add_to_token_total_supply eventually passes along the appropriate value for allow_saturation when calling add_to_token_total_supply_v0. Please double-check that any higher-level (or external) invocations also handle or forward the boolean correctly, ensuring logic consistency across the codebase.

packages/rs-drive/src/drive/tokens/system/add_to_token_total_supply/v0/mod.rs (3)

8-8: TokenAmount import is consistent with top-level changes.
This aligns with switching from u64 to TokenAmount.


93-93: New return struct captures the actual added token amount.
Returning (Vec<LowLevelDriveOperation>, TokenAmount) clarifies the operation results. Looks good for post-processing in calling layers.

Also applies to: 99-99


57-59:

✅ Verification successful

Confirm that external calls pass TokenAmount correctly.
The function signature now takes TokenAmount and returns TokenAmount. Double-check that all call sites handle or convert the value properly to avoid misunderstandings about the underlying numeric representation.

Also applies to: 64-64


🏁 Script executed:

#!/bin/bash
# Searching references to add_to_token_total_supply_add_to_operations_v0 function
rg "add_to_token_total_supply_add_to_operations_v0" -A 5 -B 5

Length of output: 3560


Confirmed proper handling of TokenAmount across calls
After reviewing the call sites in packages/rs-drive/src/drive/tokens/system/add_to_token_total_supply/v0/mod.rs, it’s clear that the TokenAmount parameter is passed into add_to_token_total_supply_add_to_operations_v0 consistently and without any unintended conversions. Both the function signature and its usage (at lines 57–59 and line 64) indicate that external calls correctly provide and receive TokenAmount, ensuring the underlying numeric representation is maintained as expected.

packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/validation.rs (4)

8-8: Added new constant for better validation of linear slope parameters.

The introduction of MAX_LINEAR_SLOPE_PARAM as a separate constant from MAX_DISTRIBUTION_PARAM allows for more precise constraints specifically for linear slope parameters.


145-146: Improved parameter naming for better readability.

Renaming the s parameter to start_step and b to starting_amount in the Linear variant significantly improves code clarity and maintainability. This descriptive naming makes the code more self-documenting.


167-173: Enhanced validation with dedicated slope parameter bounds.

The validation logic now uses the dedicated MAX_LINEAR_SLOPE_PARAM constant instead of the generic MAX_DISTRIBUTION_PARAM for the slope parameter (a). This provides more appropriate bounds specifically tailored for linear functions.


223-228: Updated function invocation with renamed parameters and contract registration step.

The code now correctly uses the renamed parameters start_step and starting_amount and also passes a new parameter (0) to the evaluate method. This suggests that the evaluation now needs a contract registration step parameter to function correctly.

packages/rs-drive/src/drive/tokens/distribution/add_pre_programmed_distribution/v0/mod.rs (2)

4-7: Imported new token distribution path constants.

Added new path constants for token distribution management, enhancing the module's capabilities to track last claims for both perpetual and pre-programmed distributions.


137-137: Modified cost estimation to use optional keys.

Changed the argument from a direct keys reference to an optional reference using Some(distribution.distributions().keys()). This change aligns with updated function signature requirements for cost estimation.

packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/add_epoch_pool_to_proposers_payout_operations/mod.rs (1)

6-6: Added BTreeMap import for improved data structure.

Imported BTreeMap from standard collections, which will be used to replace the vector of tuples in the return type.

identity_balance_holder_id: self.identity_balance_holder_id(),
mint_amount: self.mint_amount(),
allow_first_mint: false,
allow_saturation: false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

New parameter: allow_saturation added to TokenMint operation

The addition of the allow_saturation parameter (set to false by default) provides more control over token minting behavior. This is a good conservative default that prevents unexpected saturation conditions.

Can you confirm that all other implementations of TokenOperationType::TokenMint have been updated to include this new parameter? This will help ensure consistency across the codebase.


🏁 Script executed:

#!/bin/bash
# Search for other instances of TokenMint to ensure all have the allow_saturation parameter
rg -A 3 -B 3 "TokenOperationType::TokenMint" --type rust

Length of output: 3585


Ensure Consistency Across All TokenMint Implementations

The updated code in token_mint_transition.rs correctly adds the new allow_saturation parameter (defaulting to false) to the TokenMint operation. However, the search results indicate that other instances of TokenOperationType::TokenMint—specifically within:

  • packages/rs-drive/src/util/batch/drive_op_batch/token.rs
  • packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_claim_transition.rs

do not include the new allow_saturation parameter. Please update these invocations to include allow_saturation for consistency across the codebase.

Comment on lines +9 to +27
fn next_interval(&self, block_info: &BlockInfo) -> RewardDistributionMoment {
match self.distribution_type {
// If the distribution is based on block height, return the next height where emissions occur.
RewardDistributionType::BlockBasedDistribution { interval, .. } => BlockBasedMoment(
(block_info.height - block_info.height % interval).saturating_add(interval),
),

// If the distribution is based on time, return the next timestamp in milliseconds.
RewardDistributionType::TimeBasedDistribution { interval, .. } => TimeBasedMoment(
(block_info.time_ms - block_info.time_ms % interval).saturating_add(interval),
),

// If the distribution is based on epochs, return the next epoch index.
RewardDistributionType::EpochBasedDistribution { interval, .. } => EpochBasedMoment(
(block_info.epoch.index - block_info.epoch.index % interval)
.saturating_add(interval),
),
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

The next_interval method correctly returns structured reward distribution moments

The refactoring from returning raw u64 values to structured RewardDistributionMoment variants improves type safety and makes the code more self-documenting. The implementation handles all three distribution types appropriately.

However, I notice that saturating_add is still being used instead of checked_add. Based on previous feedback, consider using checked arithmetic to prevent potential overflows.

-                (block_info.height - block_info.height % interval).saturating_add(interval),
+                block_info.height.checked_sub(block_info.height % interval)
+                    .and_then(|result| result.checked_add(interval))
+                    .unwrap_or(u64::MAX),

Similar changes should be applied to the time-based and epoch-based calculations.


🏁 Script executed:

#!/bin/bash
# Check if there are any instances in the codebase where overflow might cause issues
rg "saturating_add|saturating_sub" --type rust packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/

Length of output: 2404


Action Required: Replace Saturating Arithmetic with Checked Arithmetic

The refactoring to return strongly typed RewardDistributionMoment variants is a solid improvement, enhancing type safety and clarity. However, the use of saturating_add (and related saturating_sub) in the computations might silently mask overflow issues. Based on the repository-wide usage (confirmed by the search), we should update these arithmetic operations to use checked_sub/checked_add with proper fallback logic. For example, adjust the block-based distribution calculation as follows:

-                (block_info.height - block_info.height % interval).saturating_add(interval),
+                block_info.height.checked_sub(block_info.height % interval)
+                    .and_then(|result| result.checked_add(interval))
+                    .unwrap_or(u64::MAX),

Action Items:

  • Apply the above pattern to the block-based, time-based, and epoch-based distribution calculations in packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/v0/methods.rs.
  • Review similar arithmetic operations in other related modules (e.g., in reward_distribution_moment/mod.rs and distribution_function/evaluate.rs) to ensure consistent overflow protection.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fn next_interval(&self, block_info: &BlockInfo) -> RewardDistributionMoment {
match self.distribution_type {
// If the distribution is based on block height, return the next height where emissions occur.
RewardDistributionType::BlockBasedDistribution { interval, .. } => BlockBasedMoment(
(block_info.height - block_info.height % interval).saturating_add(interval),
),
// If the distribution is based on time, return the next timestamp in milliseconds.
RewardDistributionType::TimeBasedDistribution { interval, .. } => TimeBasedMoment(
(block_info.time_ms - block_info.time_ms % interval).saturating_add(interval),
),
// If the distribution is based on epochs, return the next epoch index.
RewardDistributionType::EpochBasedDistribution { interval, .. } => EpochBasedMoment(
(block_info.epoch.index - block_info.epoch.index % interval)
.saturating_add(interval),
),
}
}
fn next_interval(&self, block_info: &BlockInfo) -> RewardDistributionMoment {
match self.distribution_type {
// If the distribution is based on block height, return the next height where emissions occur.
RewardDistributionType::BlockBasedDistribution { interval, .. } => BlockBasedMoment(
block_info.height.checked_sub(block_info.height % interval)
.and_then(|result| result.checked_add(interval))
.unwrap_or(u64::MAX),
),
// If the distribution is based on time, return the next timestamp in milliseconds.
RewardDistributionType::TimeBasedDistribution { interval, .. } => TimeBasedMoment(
block_info.time_ms.checked_sub(block_info.time_ms % interval)
.and_then(|result| result.checked_add(interval))
.unwrap_or(u64::MAX),
),
// If the distribution is based on epochs, return the next epoch index.
RewardDistributionType::EpochBasedDistribution { interval, .. } => EpochBasedMoment(
block_info.epoch.index.checked_sub(block_info.epoch.index % interval)
.and_then(|result| result.checked_add(interval))
.unwrap_or(u64::MAX),
),
}
}

Comment on lines +81 to 92
Self::try_from_borrowed_token_claim_transition_with_contract_lookup(
drive,
owner_id,
&value,
approximate_without_state_for_costs,
transaction,
block_info,
user_fee_increase,
get_data_contract,
platform_version,
None,
)?;

let base_action = match base_action_validation_result.is_valid() {
true => base_action_validation_result.into_data()?,
false => {
let bump_action = BumpIdentityDataContractNonceAction::from_token_base_transition(
base,
owner_id,
user_fee_increase,
);
let batched_action =
BatchedTransitionAction::BumpIdentityDataContractNonce(bump_action);

return Ok((
ConsensusValidationResult::new_with_data_and_errors(
batched_action.into(),
base_action_validation_result.errors,
),
fee_result,
));
}
};

Ok((
BatchedTransitionAction::TokenAction(TokenTransitionAction::ClaimAction(
TokenClaimTransitionActionV0 {
base: base_action,
amount: 0, //todo
distribution_info: todo!(),
public_note,
}
.into(),
))
.into(),
fee_result,
))
)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Refactor to try_from_borrowed_token_claim_transition_with_contract_lookup.
Eliminates data-copy overhead and unifies the logic in a single place.

@QuantumExplorer QuantumExplorer added this to the v2.0.0 milestone Mar 13, 2025
@QuantumExplorer
Copy link
Member Author

Merging this in to make a pre-release. @shumkov please review when you can.

@QuantumExplorer QuantumExplorer merged commit 5849244 into v2.0-dev Mar 13, 2025
78 checks passed
@QuantumExplorer QuantumExplorer deleted the feat/token_distribution4 branch March 13, 2025 16:29
lklimek added a commit that referenced this pull request Mar 17, 2025
chore: update to latest dash core 37 (#2483)

feat(platform)!: token advanced distribution and updates (#2471)

fix: token history contract (#2474)

Co-authored-by: Ivan Shumkov <[email protected]>
Co-authored-by: QuantumExplorer <[email protected]>

fix(drive): using new rust dash core methods for reversed quorum hash to maintain backwards compatibility (#2489)

feat: more granular integer document property types (#2455)

Co-authored-by: Quantum Explorer <[email protected]>

docs: update comment for data contract code range (#2476)

feat: validate token name localizations (#2468)

feat(sdk): get identity by non-unique keys

build(deps): update grovedb to current develop

test: test identity by non-unique pubkey hashes

fix(sdk): dash core client fails to get quorum

chore: minor fixes

test(drive-abci): identity by non-unique pubkey start after

chore: minor changes to verify

feat(sdk): token and group queries (#2449)

chore: revert limit 1 => limit none

chore: add non-unique key to test identities

test(sdk): test vectors for test_fetch_identity_by_non_unique_public_keys

fix(platform)!: token distribution fixes and tests (#2494)

chore(platform): bump to version 2.0.0-dev.1 (#2495)

test: update assertion

fix(sdk): make some things public (#2496)

feat(platform): require token for document actions (#2498)

fix: data contract proof doesn't work  with new auto fields (#2501)
lklimek added a commit that referenced this pull request Mar 18, 2025
chore: update to latest dash core 37 (#2483)

feat(platform)!: token advanced distribution and updates (#2471)

fix: token history contract (#2474)

Co-authored-by: Ivan Shumkov <[email protected]>
Co-authored-by: QuantumExplorer <[email protected]>

fix(drive): using new rust dash core methods for reversed quorum hash to maintain backwards compatibility (#2489)

feat: more granular integer document property types (#2455)

Co-authored-by: Quantum Explorer <[email protected]>

docs: update comment for data contract code range (#2476)

feat: validate token name localizations (#2468)

feat(sdk): get identity by non-unique keys

build(deps): update grovedb to current develop

test: test identity by non-unique pubkey hashes

fix(sdk): dash core client fails to get quorum

chore: minor fixes

test(drive-abci): identity by non-unique pubkey start after

chore: minor changes to verify

feat(sdk): token and group queries (#2449)

chore: revert limit 1 => limit none

chore: add non-unique key to test identities

test(sdk): test vectors for test_fetch_identity_by_non_unique_public_keys

fix(platform)!: token distribution fixes and tests (#2494)

chore(platform): bump to version 2.0.0-dev.1 (#2495)

test: update assertion

fix(sdk): make some things public (#2496)

feat(platform): require token for document actions (#2498)

fix: data contract proof doesn't work  with new auto fields (#2501)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant